fix: retry HTTP download in install.ps1 on transient failure - #183
fix: retry HTTP download in install.ps1 on transient failure#183juhovainio wants to merge 1 commit into
Conversation
Invoke-WebRequest had no retry logic, so a single transient connection failure (e.g. connection reset, or the server-side listener not yet ready) would abort the install outright. This is the exact failure signature behind the intermittent lifecycle-windows-http-install E2E flake, and it also affects real production HTTP downloads. The expectations.toml flaky mechanism was considered but doesn't fit here: it only tolerates an unexpected pass of a known-bug xfail scenario, not an unexpected fail of an expect-pass scenario, so it can't be reused to tolerate this failure mode without a design change to the expectation system itself. Save-File now retries the Invoke-WebRequest call up to 3 times with a short increasing backoff before failing. The local file:// (Copy-Item) path is untouched since it isn't networked and isn't implicated in the flake. Signed-off-by: Juho Vainio <juho.vainio@amd.com>
rominf
left a comment
rominf
left a comment
There was a problem hiding this comment.
Holding this one — I don't think the retry is fixing the bug we're actually seeing, and I should disclose up front that the alternative I'm pointing at is my own PR, so weigh this accordingly.
The failure looks like a test-server bug, not a network transient
The signature in #173 is specific: the archive downloads fine, then the immediately following .sha256 request to the same loopback server fails with a bare An error occurred while sending the request — transport level, no HTTP status.
The server that scenario points at is hand-rolled on a raw TcpListener in tests/e2e-cucumber/tests/e2e/lifecycle_steps.rs, set non-blocking so the accept loop can poll a stop flag. On Windows — and not on Linux, which is why this is Windows-only — accept() returns a socket that inherits the listener's non-blocking mode. If the request bytes haven't landed yet, the first read() returns WouldBlock, the handler writes no response and drops the socket, and the client sees exactly that error. It also explains why it's the second request that fails: it's issued the instant the first download completes, with no process setup ahead of it to win the race.
#174 replaces that server with axum + tower-http::ServeDir and has ten consecutive green Windows E2E runs behind it. It's been open since 3 Aug. If it lands, this retry loop becomes permanent complexity in the shipped installer, masking a test bug that no longer exists.
On the retry itself
Retrying real HTTP downloads in an installer is defensible on its own merits — field transients are real. But that's a separate proposal that should stand on its own evidence, not ride in as a flake fix. As written, the PR description hedges on the cause ("a connection reset, or the listener not yet ready") and cites no log, no reproduction and no repeated-run evidence tying the retry to the observed failure.
One thing worth recording either way, since it comes up whenever this is revisited: Invoke-WebRequest's built-in -MaximumRetryCount is not an option here. Aside from needing PowerShell 6+ while this script still supports Windows PowerShell 5.1 (install.ps1:163, :607), it only retries on received HTTP failure responses — it never retries pre-response transport exceptions, which is exactly this failure class. A hand-rolled loop is the right shape if we do want retries.
Loop review, in case it goes ahead independently
No blockers, but:
- It retries permanent failures too — a 404 or 401 burns all three attempts and 750ms before failing. Worth restricting to transport errors and 5xx/429.
- The last-attempt
Start-Sleepis unreachable only becauseFailcallsexit 1first. That's incidental rather than structural; a comment or a restructure would make it hold on purpose. - Partial
-OutFilewrites are cleaned on every failed attempt,returninsidetryis correct, and the loop can't fall through — those all check out.
Suggest we let #174 land first and then decide whether installer retries are still wanted on their own terms.
2 participants
Summary
Save-File'sInvoke-WebRequestcall had no retry logic: a single transient connection failure (e.g. a connection reset, or the loopback/server-side listener not yet fully ready) aborted the install outright.lifecycle-windows-http-installE2E flake seen onmainand on PR test(e2e): tolerate intermittent EAI-7423 XPASS on the Strix Halo Ubuntu lane (EAI-7853) #182's merge queue attempt ("An error occurred while sending the request"), and it also affects real-world production HTTP downloads, not just the test's loopback server.Save-Filenow retries theInvoke-WebRequestcall up to 3 times with a short increasing backoff (250ms/500ms) before failing. The localfile://(Copy-Item) branch is untouched since it isn't networked and isn't implicated in the flake.Why not use
expectations.toml'sflakymechanism instead?I looked into marking the scenario as flaky via the existing xfail matrix, but it doesn't fit this failure shape:
flakyonly tolerates an unexpected pass of a scenario that's expected to fail as a known bug (an XPASS).lifecycle-windows-http-installis an ordinaryExpectPassscenario that intermittently fails outright — the opposite case, which the reconciliation logic intests/e2e-cucumber/tests/e2e.rsalways treats as a fatalunexpected_fail, with no existing tolerance path.Reusing/extending that mechanism to also cover unexpected-fail-of-expect-pass was flagged in PR #182's review discussion as a bigger design decision (a quarantine marker with an expiry was suggested as the better long-term shape) that shouldn't be decided ad hoc here. A genuine reliability fix at the actual failure site is more proportionate, and improves the real installer besides.
Test plan
lifecycle-windows-http-install, should pass reliably (or at minimum no longer show this exact failure signature if it's hit again transiently, since it'll now retry).install.ps1stays clean.pwshavailable in this (Linux/WSL) dev environment, and there's no Pester/unit suite forinstall.ps1— it's only exercised via the Windows E2E cucumber scenarios in CI.